Skip to content

feat: adapt to eloqstore local standby feature#451

Merged
thweetkomputer merged 7 commits intoeloqdata:mainfrom
thweetkomputer:feat-eloqstore-local-standby-zc
Mar 17, 2026
Merged

feat: adapt to eloqstore local standby feature#451
thweetkomputer merged 7 commits intoeloqdata:mainfrom
thweetkomputer:feat-eloqstore-local-standby-zc

Conversation

@thweetkomputer
Copy link
Collaborator

@thweetkomputer thweetkomputer commented Mar 4, 2026

Summary by CodeRabbit

  • Chores
    • Clarified configuration documentation for store shard paths: multi-path format and optional per-path weights (format: path1,path2,...[,pathN][:weight1,weight2,...,weightN]); omitted weights default to disk-capacity-based weighting
    • Added configuration option eloq_store_standby_max_concurrency=100 to control maximum concurrent standby rsync/ssh jobs
    • No functional or public API changes in this release

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Updated the data_substrate subproject commit pointer and extended comments plus new config key in eloqkv.ini to document per-store-path weighting and a standby concurrency setting; no public/exported API declarations or runtime behavior changes.

Changes

Cohort / File(s) Summary
Submodule Update
data_substrate
Bumped subproject commit reference from 500ffa3... to 95b3d72...; metadata-only update with no observable functional or public API changes.
Configuration / Docs
eloqkv.ini
Expanded [store] shard data path comment to specify format path1,path2,...[,pathN][:weight1,weight2,...,weightN] (weights optional, default: disk-capacity-based weighting) and added eloq_store_standby_max_concurrency=100 config entry; documentation/config additions only, no logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

trigger-ci

Suggested reviewers

  • liunyl

Poem

🐰
I nudged a pointer, inked a line,
Clarified weights so paths align.
A quiet hop, a config cheer,
Submodule set — CI draws near!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: adapt to eloqstore local standby feature' directly reflects the main changes in the PR: adding the eloq_store_standby_max_concurrency configuration option and expanding related documentation for the EloqStore standby feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
eloqkv.ini (1)

179-194: Move LOCAL STANDBY configuration block outside CLOUD MODE section; these configs are not yet implemented in the codebase.

The LOCAL STANDBY block is placed within the "CLOUD MODE CONFIGURATION" section (lines 136-140), creating confusion since line 180 explicitly states to "keep this disabled in cloud mode." Additionally, these configuration keys are not referenced anywhere in the codebase, indicating this is forward-looking documentation for a feature not yet implemented.

To improve clarity:

  1. Move the LOCAL STANDBY block outside the "CLOUD MODE CONFIGURATION" section, or add a clear subsection header like # >>>>>>>>>>>>> LOCAL STANDBY (NON-CLOUD) <<<<<<<<<<<<<
  2. Consider adding validation guidance for when the feature is implemented:
    • What happens if eloq_store_standby_master_snapshot_roots count doesn't match the actual snapshot roots on the master?
    • What happens if eloq_store_standby_master_store_path_weights count doesn't match the master's store path list?
  3. Clarify the host_name:/path format when the host includes a port (e.g., host:port:/path vs host:/path)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eloqkv.ini` around lines 179 - 194, Move the LOCAL STANDBY configuration
block (keys eloq_store_enable_local_standby, eloq_store_standby_master_path,
eloq_store_standby_master_snapshot_roots,
eloq_store_standby_master_store_path_weights) out of the "CLOUD MODE
CONFIGURATION" section or prepend a distinct subsection header like "LOCAL
STANDBY (NON-CLOUD)" to avoid implying cloud compatibility; mark these keys as
not yet implemented in the codebase, and add short TODO/validation notes
indicating required behavior once implemented (e.g., how mismatched counts for
eloq_store_standby_master_snapshot_roots vs master snapshot roots and
eloq_store_standby_master_store_path_weights vs master store paths should be
handled), and clarify the host:path format rules including how to represent a
host with a port (explicitly state expected syntax such as
username@host:port:/absolute/path or an alternative).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@eloqkv.ini`:
- Around line 179-194: Move the LOCAL STANDBY configuration block (keys
eloq_store_enable_local_standby, eloq_store_standby_master_path,
eloq_store_standby_master_snapshot_roots,
eloq_store_standby_master_store_path_weights) out of the "CLOUD MODE
CONFIGURATION" section or prepend a distinct subsection header like "LOCAL
STANDBY (NON-CLOUD)" to avoid implying cloud compatibility; mark these keys as
not yet implemented in the codebase, and add short TODO/validation notes
indicating required behavior once implemented (e.g., how mismatched counts for
eloq_store_standby_master_snapshot_roots vs master snapshot roots and
eloq_store_standby_master_store_path_weights vs master store paths should be
handled), and clarify the host:path format rules including how to represent a
host with a port (explicitly state expected syntax such as
username@host:port:/absolute/path or an alternative).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59fe4e5f-2afb-4dcb-9cde-7ba3cf5bc8d6

📥 Commits

Reviewing files that changed from the base of the PR and between af77deb and 42fc632.

📒 Files selected for processing (2)
  • data_substrate
  • eloqkv.ini

@thweetkomputer thweetkomputer force-pushed the feat-eloqstore-local-standby-zc branch from 42fc632 to 7a511ed Compare March 5, 2026 06:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
eloqkv.ini (1)

192-194: Make standby weight cardinality explicit.

Please explicitly state that eloq_store_standby_master_store_path_weights count must equal eloq_store_standby_master_store_paths count (similar to Line 96-97), to prevent misconfiguration ambiguity.

Suggested wording
-# [LOCAL STANDBY OPTIONAL] Store path weights for the standby master, separated by ','.
-# Use the same ordering as the standby master's store path list.
+# [LOCAL STANDBY OPTIONAL] Store path weights for the standby master, separated by ','.
+# The number of weights must match eloq_store_standby_master_store_paths.
+# Use the same ordering as the standby master's store path list.
 # eloq_store_standby_master_store_path_weights=
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eloqkv.ini` around lines 192 - 194, Update the comment for
eloq_store_standby_master_store_path_weights to explicitly state that the number
of comma-separated weights must match the number of entries in
eloq_store_standby_master_store_paths (i.e., they must have the same
cardinality), mirroring the guidance used earlier for the primary store paths;
mention the requirement clearly so users know to provide one weight per standby
store path and note that omission or mismatch is invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@eloqkv.ini`:
- Around line 192-194: Update the comment for
eloq_store_standby_master_store_path_weights to explicitly state that the number
of comma-separated weights must match the number of entries in
eloq_store_standby_master_store_paths (i.e., they must have the same
cardinality), mirroring the guidance used earlier for the primary store paths;
mention the requirement clearly so users know to provide one weight per standby
store path and note that omission or mismatch is invalid.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e58a93d-a5e1-4bfc-8810-6f6fca15004f

📥 Commits

Reviewing files that changed from the base of the PR and between 42fc632 and 7a511ed.

📒 Files selected for processing (2)
  • data_substrate
  • eloqkv.ini
✅ Files skipped from review due to trivial changes (1)
  • data_substrate

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eloqkv.ini`:
- Around line 176-183: The LOCAL STANDBY options
(eloq_store_enable_local_standby, eloq_store_standby_master_addr) are
ambiguously placed and under-documented: move or re-section them out of the
"CLOUD MODE CONFIGURATION" block (or add a clear subsection header "LOCAL
STANDBY (optional, only for non-cloud or advanced use)") and update both entries
to match file conventions—document eloq_store_enable_local_standby as a boolean
with allowed values (e.g., "true/on" to enable, "false/off" to disable) and a
short note that it should normally remain disabled in managed cloud mode;
document eloq_store_standby_master_addr with explicit format examples and rules
(username@host[:port], supporting hostname or IP for host_addr, port optional;
show example like "replica@10.0.0.5:5432" and state behavior when set: standby
will sync from that master instead of cloud storage).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4afd560d-8855-4a93-98ef-7d33145b497c

📥 Commits

Reviewing files that changed from the base of the PR and between 7a511ed and 74c798f.

📒 Files selected for processing (2)
  • data_substrate
  • eloqkv.ini
✅ Files skipped from review due to trivial changes (1)
  • data_substrate

@thweetkomputer thweetkomputer force-pushed the feat-eloqstore-local-standby-zc branch from 06fe140 to 12604fc Compare March 16, 2026 02:44
@thweetkomputer thweetkomputer force-pushed the feat-eloqstore-local-standby-zc branch from 6d0ec13 to 85c5897 Compare March 16, 2026 07:25
fix

wip

fix

wip

update

config

update

prolang timeout

update test

fix

update

update

update

update

update

update
@thweetkomputer thweetkomputer force-pushed the feat-eloqstore-local-standby-zc branch from 91bee0f to 136ff33 Compare March 16, 2026 16:59
@thweetkomputer thweetkomputer merged commit 1b9a997 into eloqdata:main Mar 17, 2026
4 of 5 checks passed
@thweetkomputer thweetkomputer deleted the feat-eloqstore-local-standby-zc branch March 17, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants